wayland/pointer_constraints: call remove_constraint when pointer leave surface by skygrango · Pull Request #2107 · Smithay/smithay

skygrango · 2026-07-22T03:06:06Z

Description

related: #2027 (comment)

The implementation ensures that remove_constraint() is invoked when the pointer leaves the surface, allowing the last saved cursor position hint to be applied.

This both improves the expected behavior of Lifetime::Persistent and ensures that pointer focus changes are handled immediately.

Checklist

I agree to smithay's Developer Certificate of Origin.

skygrango · 2026-07-22T03:07:44Z

@YaLTeR Does this meet your needs ?

skygrango · 2026-07-22T04:30:20Z

Introducing the PointerConstraintsHandler trait significantly increases the scope of the changes. Perhaps we should move remove_constraint() out of PointerConstraintsHandler

YaLTeR · 2026-07-22T07:18:32Z

+            if deactivated {
+                data.remove_constraint(self, &pointer);
+            }


I would imagine that this code should go inside deactivate() itself to avoid putting it into every caller?

Or I guess data is inaccessible from there? Hmm, not sure how to structure this

I've updated the signature of deactivate(), which I believe addresses your suggestion.

However, the downside is that PointerConstraintsHandler has now become a trait dependency of PointerTarget.

That looks like the change that I had in mind, yeah.

It also doesn't seem like the repeated remove_constraint() call from destroyed() is a problem, because the constraint is required to be unique per surface, so the surface must destroy the previous constraint before making a new one, so it shouldn't be possible for a repeated remove_constraint() call to operate on a cursor position hint from a different constraint. Is my reasoning correct here?

For Lifetime::Oneshot, explicitly calling deactivate() does result in one extra invocation of remove_constraint(). However, I don't think this has any practical impact. The compositor should ensure that only a single surface's cursor position hint is stored at any given time(per seat), and that the hint is consumed after it has been been applied. Consequently, the second invocation becomes a no-op because there is no cursor position hint remaining to apply.

Yes, and I am thinking through whether it's possible to end up with the incorrect stored hint through overlapping pointer constraint objects, and so far I don't see any way.

skygrango · 2026-07-22T10:12:59Z

I moved remove_constraint() to SeatHandler, which significantly reduces the amount of trait boilerplate and maintenance.

Since remove_constraint() may be called while with_pointer_constraint() is active, the PointerConstraint is passed to the compositor so it can inspect the RegionAttributes without re-entering with_pointer_constraint().

Drakulix · 2026-07-23T13:29:52Z

        self.backend_data.update_led_state(led_state)
    }
+
+    fn remove_constraint(


Urgh, I am not sure I like the move here. So if you don't expose the constrains-protocol, you still have to implement this function? That makes no sense.

Drakulix · 2026-07-23T13:37:42Z

            with_pointer_constraint(self, &pointer, |constraint| {
                if let Some(constraint) = constraint {
-                    constraint.deactivate();
+                    constraint.deactivate(data, &self, &pointer);


So the issue is this call, right? (This would be so easy, if we had specialization.)

I would honestly rather have the PointerTarget impl for WlSurface require the PointerContraintsHandler, than remove_constraint to be part of the SeatHandler, if nobody has a better idea.

how do you think about

/// Callback that returns a reference to the pointer constraints handler, if implemented. fn pointer_constraints_handler(&mut self) -> Option<&mut dyn PointerConstraintsHandler<Self>> { None }

for SeatHandler

No please. The SeatHandler should not expose anything related to pointer constraints. If our WlSurface implementation of PointerTarget needs it, just add a requirement for the PointerConstraintsHandler.

skygrango · 2026-07-25T04:32:57Z

@Drakulix Following your suggestion, I added PointerConstraintsHandler to PointerTarget. Could you please let me know if this is good enough? I'll clean up the Git history later.

Drakulix

LGTM

I'll clean up the Git history later.

I can simply squash the commits on merge, if you want.

YaLTeR · 2026-08-08T16:26:08Z

I think this just needs to be merged now?

…eactivate

skygrango · 2026-08-09T09:35:38Z

Thank you for your review. This can be merged now.

YaLTeR · 2026-08-11T06:32:13Z

Uh oh, so there's actually a deadlock problem. PointerHandle::motion() locks the pointer inner, then calls <WlSurface as PointerTarget>::leave() which then calls compositor's remove_constraint(). The compositor is expected to call pointer.set_location() inside, which then again tries locking the pointer handle, resulting in a deadlock.

YaLTeR · 2026-08-11T06:41:27Z

Hm, I guess it's debatable whether you even want to warp the pointer if the constraint was broken by pointer leaving the surface. Is there a way to find out in remove_constraint() whether this is the case? Then just skip set_location.

YaLTeR · 2026-08-11T06:44:21Z

Perhaps need to pass a new parameter like enum RemoveReason { LeftSurface, Deactivated, Destroyed }?

YaLTeR · 2026-08-11T06:52:37Z

Luckily with the current code it happens to be possible to tell if I'm not mistaken:

        if pointer.last_enter().is_some() {
            pointer.set_location(target);
        }

Since pointer.wl_pointer.leave(self, serial, time); is called right before constraint.deactivate(data, self, &pointer);, then by the time remove_constraint() is called, last_enter() is already cleared out. And it doesn't lock the inner handle.

And when the constraint is deactivated by the client, last_enter() will still point at that client, so the pointer will warp fine.

skygrango force-pushed the fix_pointer_constraint branch 3 times, most recently from 7ea2362 to b665e57 Compare July 22, 2026 03:42

YaLTeR reviewed Jul 22, 2026

View reviewed changes

skygrango force-pushed the fix_pointer_constraint branch 7 times, most recently from c731f38 to d551a7f Compare July 22, 2026 10:07

Drakulix reviewed Jul 23, 2026

View reviewed changes

skygrango force-pushed the fix_pointer_constraint branch 5 times, most recently from 29c3be2 to 43ee475 Compare July 25, 2026 04:22

Drakulix approved these changes Jul 27, 2026

View reviewed changes

wayland/pointer_constraints: call remove_constraint when constraint d…

46a8751

…eactivate

skygrango force-pushed the fix_pointer_constraint branch from 43ee475 to 46a8751 Compare August 9, 2026 09:32

YaLTeR mentioned this pull request Aug 10, 2026

Update smithay and implement tablet grabs niri-wm/niri#4420

Merged

Drakulix approved these changes Aug 10, 2026

View reviewed changes

Drakulix merged commit a12d486 into Smithay:master Aug 10, 2026
14 checks passed

Vantrongs mentioned this pull request Aug 10, 2026

Keep pointer constraints in sync with keyboard focus niri-wm/niri#4411

Open

YaLTeR mentioned this pull request Aug 11, 2026

Deadlock when pointer focus leaves a constrained fullscreen Xwayland surface niri-wm/niri#4428

Closed

YaLTeR mentioned this pull request Aug 11, 2026

anvil: Fix deadlock in remove_constraint() #2129

Merged

1 task

Uh oh!

Conversation

skygrango commented Jul 22, 2026

Description

Checklist

Uh oh!

skygrango commented Jul 22, 2026

Uh oh!

skygrango commented Jul 22, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skygrango commented Jul 22, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skygrango commented Jul 25, 2026

Uh oh!

Drakulix left a comment

Choose a reason for hiding this comment

Uh oh!

YaLTeR commented Aug 8, 2026

Uh oh!

skygrango commented Aug 9, 2026

Uh oh!

Uh oh!

YaLTeR commented Aug 11, 2026

Uh oh!

YaLTeR commented Aug 11, 2026

Uh oh!

YaLTeR commented Aug 11, 2026

Uh oh!

YaLTeR commented Aug 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants